-
Couldn't load subscription status.
- Fork 1.9k
[Kernel] Add snapshot construction benchmarking specification #5339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| if (workloadSpec.getVersion() != null) { | ||
| builder.atVersion(workloadSpec.getVersion()); | ||
| } | ||
| return builder.build(engine); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you call snapshot.getVersion, or cast it to SnapshotImpl and then call .getMetadata or .getProtocol? and maybe even print it out as a simple validation? or is that out of scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this is part of the inner loop of the benchmark, I want to avoid any logging. Moreover, due to repeat runs, logging this can actually get quite noisy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one area where this type of logging can be introduced is in the executeAsTest when that eventually comes in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- Asked a question about some tiny validation. I defer to you. And clean up as needed. Thanks!
Which Delta project/connector is this regarding?
Description
This introduces the snapshot construction spec and workload runner. These are used to measure the time it takes to construct a snapshot and perform protocol & metadata phase.
The snapshot construction spec takes an optional version.
How was this patch tested?
Does this PR introduce any user-facing changes?